feat: add chainbench grpc subcommand for Solana Yellowstone gRPC benchmarks#121
feat: add chainbench grpc subcommand for Solana Yellowstone gRPC benchmarks#121CSFeo wants to merge 2 commits into
chainbench grpc subcommand for Solana Yellowstone gRPC benchmarks#121Conversation
📝 WalkthroughWalkthroughChangesThe PR adds gRPC benchmark integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant grpc_wrapper
participant chainbench_grpc
CLI->>grpc_wrapper: invoke grpc mode with arguments
grpc_wrapper->>grpc_wrapper: locate chainbench-grpc on PATH
grpc_wrapper->>chainbench_grpc: execute mode and forwarded arguments
chainbench_grpc-->>grpc_wrapper: return exit code
grpc_wrapper-->>CLI: exit with binary return code
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
chainbench/grpc.py (1)
31-31: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCatch
OSErrorfor robust binary execution.While
shutil.whichensures the binary exists and is marked as executable,subprocess.runcan still raise anOSError(e.g.,ENOEXECdue to a CPU architecture mismatch if a user downloads the wrong local binary). Catching this exception and raising aclick.ClickExceptionprovides a cleaner user experience than a raw Python traceback.🛠️ Proposed refactor to handle execution errors gracefully
- completed = subprocess.run([exe, mode, *args]) + try: + completed = subprocess.run([exe, mode, *args]) + except OSError as e: + raise click.ClickException(f"Failed to execute `{exe}`: {e}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@chainbench/grpc.py` at line 31, Update the subprocess.run invocation in the binary execution flow to catch OSError, including execution failures such as incompatible binaries, and raise a click.ClickException with a clear error message instead of exposing the raw traceback. Preserve the existing command arguments and successful execution behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@chainbench/grpc.py`:
- Line 31: Update the subprocess.run invocation in the binary execution flow to
catch OSError, including execution failures such as incompatible binaries, and
raise a click.ClickException with a clear error message instead of exposing the
raw traceback. Preserve the existing command arguments and successful execution
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2d0096b-3629-490d-81d8-a08a120d0d99
📒 Files selected for processing (4)
DockerfileREADME.mdchainbench/grpc.pychainbench/main.py
…nchmarks Integrate the chainbench-grpc tool as a CLI invocation inside the ChainBench Docker image (per the Solana gRPC benchmarking initiative). - chainbench/grpc.py: `chainbench grpc <race|latency|throughput|slots|full>` Click subgroup. Thin pass-through — shells out to the bundled `chainbench-grpc` binary, forwards stdout/stderr + exit code; flags after the mode go straight to the binary (it owns its own validation and --help). Clear error if not on PATH. - chainbench/main.py: register the subgroup. - Dockerfile: Rust build stage builds chainbench-grpc from a pinned tag (v0.4.0) and copies the binary onto PATH in the prod image (protoc is vendored). - README: document the new `grpc` command. Tested: black/isort/flake8/mypy clean; the subcommand shells out to the real binary end to end (verified locally), with correct exit codes and a graceful "binary not found" message.
64da86e to
7e40a12
Compare
|
@erwin-wee could you take a look when you have a moment? 🙏 This adds a Verified end-to-end: built the image and ran |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
chainbench/grpc.py (1)
31-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
execfor a true pass-through wrapper.
subprocess.runcan handle ordinary exit codes, but signals such as Ctrl-C may be handled by the Python/Click parent instead of preserving the benchmark process semantics. After the existence check,os.execv(exe, [exe, mode, *args])would inherit stdio, signals, and the child’s exit status directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@chainbench/grpc.py` around lines 31 - 32, Replace the subprocess.run and sys.exit flow in the wrapper entry point with os.execv, passing exe as both the executable and argv[0] followed by mode and args, so the wrapper directly preserves the benchmark process’s stdio, signals, and exit status.Dockerfile (1)
6-8: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse an immutable commit instead of a mutable tag.
v0.4.0can be retagged, andCHAINBENCH_GRPC_REFis overrideable at build time. Resolve the release to a full commit SHA (and verify it in CI) so production images remain reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 6 - 8, Update the Dockerfile’s CHAINBENCH_GRPC_REF clone configuration to use the full immutable commit SHA for the intended v0.4.0 release instead of a mutable tag, and prevent build-time overrides from changing that pinned revision. Ensure CI verifies the resolved commit matches the expected SHA before producing production images.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 35-36: Create a dedicated non-root runtime user in the production
image, ensure it can access the required application files and binaries, and add
the user switch before the existing ENTRYPOINT. Keep the chainbench-grpc PATH
setup unchanged while ensuring the production CLI runs without root privileges.
---
Nitpick comments:
In `@chainbench/grpc.py`:
- Around line 31-32: Replace the subprocess.run and sys.exit flow in the wrapper
entry point with os.execv, passing exe as both the executable and argv[0]
followed by mode and args, so the wrapper directly preserves the benchmark
process’s stdio, signals, and exit status.
In `@Dockerfile`:
- Around line 6-8: Update the Dockerfile’s CHAINBENCH_GRPC_REF clone
configuration to use the full immutable commit SHA for the intended v0.4.0
release instead of a mutable tag, and prevent build-time overrides from changing
that pinned revision. Ensure CI verifies the resolved commit matches the
expected SHA before producing production images.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35b52e6a-9b40-420e-91d1-a1daf4587b57
📒 Files selected for processing (4)
DockerfileREADME.mdchainbench/grpc.pychainbench/main.py
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- chainbench/main.py
| # Bundle the chainbench-grpc binary on PATH (used by `chainbench grpc`). | ||
| COPY --from=grpc /src/target/release/chainbench-grpc /usr/local/bin/chainbench-grpc |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Run the production image as a non-root user.
The new binary is added to an image that still defaults to root. Create a dedicated runtime user and switch to it before ENTRYPOINT; otherwise vulnerabilities in the CLI, benchmark binary, or dependencies have root privileges.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 35 - 36, Create a dedicated non-root runtime user in
the production image, ensure it can access the required application files and
binaries, and add the user switch before the existing ENTRYPOINT. Keep the
chainbench-grpc PATH setup unchanged while ensuring the production CLI runs
without root privileges.
Source: Linters/SAST tools
1 participant
What
Adds a
chainbench grpcsubcommand for benchmarking Solana Yellowstone (Geyser)gRPC endpoints, and bundles the
chainbench-grpctool into the Docker image so it works out of the box.
How it works
chainbench/grpc.py— a Click subgroup with the five modes (race,latency,throughput,slots,full). It's a thin pass-through: itsubprocess-invokes the bundledchainbench-grpcbinary, forwards stdout /stderr and the exit code, and passes flags after the mode straight to the
binary (which owns its own validation and
--help). If the binary isn't onPATHit raises a clearClickException.chainbench/main.py— registers the subgroup (+4lines).Dockerfile— a Rust build stage compileschainbench-grpcfrom a pinnedtag (
v0.4.0;protocis vendored by the crate) and copies the binary to/usr/local/binin the prod image.README.md— documents the new command.Testing
black --check,isort --check,flake8,mypy— clean on the changed files.chainbench-grpcbinary onPATH, and confirmedchainbench grpc <mode>shells out to it (correct output + exit codes; graceful "not found" message
when the binary is absent).
Notes for maintainers
CSFeo/chainbench-grpcrepo, pinned to tag
v0.4.0via theCHAINBENCH_GRPC_REFbuild arg.chainbench-grpcunder thechainstacklabsorg, and the published image / Docker Hub ownership.
grpccommand output is the tool's own (console/JSON/CSV/HTML); it is notnormalized into the Locust report format.
Summary by CodeRabbit
New Features
chainbench grpccommand with sub-modes:race,latency,throughput,slots, andfull.chainbench-grpcexecutable for use viachainbench grpc.Documentation